fix: resolve SonarQube S1192 and S3776 issues in discussions.go#87
Open
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
Open
fix: resolve SonarQube S1192 and S3776 issues in discussions.go#87devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
Conversation
- S1192: Extract duplicated string literals into constants (descRepoOwner, descRepoName, errGQLClientFmt, categoryLabelFmt) - S3776: Reduce cognitive complexity of ListDiscussions by extracting discussionNode type, mapDiscussionNodesToIssues helper, and separate query functions for with/without category filter Co-Authored-By: Shannon Hittson <shannon.hittson@cognition.ai>
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes: SonarQube issues
go:S1192(String literals should not be duplicated) andgo:S3776(Cognitive Complexity too high) inpkg/github/discussions.go.Summary
Addresses two High-severity SonarQube findings in
discussions.go, scoped to a single file change:S1192 – Duplicated string literals: Extracted four repeatedly-used literals into package-level constants (
descRepoOwner,descRepoName,errGQLClientFmt,categoryLabelFmt). Each was duplicated 3–4× across the four tool functions in this file.S3776 – Cognitive Complexity of
ListDiscussions(27 → well under 15): The original function contained two near-identical branches (with/without category filter), each with inline struct definitions, query execution, and node-to-Issue mapping. Refactored by extracting:discussionNode– shared struct type for GraphQL response nodesqueryDiscussionsWithCategory/queryDiscussionsWithoutCategory– query helpersmapDiscussionNodesToIssues– shared mapping logicImportant review items
nullJSON when no discussions were found (nil slice viavar discussions []*github.Issue). The new code returns[](empty slice viamake). This is arguably more correct but is a subtle contract change worth verifying downstream consumers can handle.discussionNodetype with itsgraphql:"category"andgraphql:"url"tags is correctly resolved by thegithubv4client when used inside the wrapper query structs inqueryDiscussionsWithCategory/queryDiscussionsWithoutCategory.descRepoOwner,descRepoName, etc. are unexported (lowercase) and local to thegithubpackage. If other files in this package use the same literals, these constants could be reused — but this PR intentionally limits changes to one file.Tradeoffs
queryDiscussionsWithCategory/queryDiscussionsWithoutCategory) were kept instead of a single parameterized function because the GraphQL query struct tags differ (discussions(first: 100, categoryId: $categoryId)vsdiscussions(first: 100)), making compile-time struct unification non-trivial.Link to Devin run: https://app.devin.ai/sessions/cb8670bcf6e74341a4ed0784765549ec
Requested by: Shannon Hittson (@shannonhittson-eng)